feat(policies): expiry + scope lifecycle controls (token-optimization R2) - #1893

Summary

R2 of the token-optimization program (research: library /engineering/research/token-optimization-research.md §3.3 and §8/R2; idea 01M0QGZQ15WE9DDQENGV6S9XZ5). Follows R1 (#1891), which removed the duplicated knowledgeContext/policyContext arrays.

getActivePolicies injects every active policy into every session, unranked and with no shelf life. This project has 81 active policies against a cap of 100, and several of them describe work that finished weeks ago — "Use Codex 5.5 High Chat VMs for current reliability workflow" names a 2026-08-21 workstream that is over. They will keep loading into every future session's opening turn until a human notices. There was no way for an agent capturing a genuinely temporary constraint to mark it as temporary: add_policy had no expiry and no scope, so every policy was implicitly permanent.

Two additive columns fix that:

• expires_at INTEGER (nullable) — the filter. NULL means "never expires", which is exactly today's behaviour, so all 81 existing rows are unaffected by construction.
• scope TEXT NOT NULL DEFAULT 'always' ('always' | 'task') — the discriminator, backfilled to 'always'.

The load-bearing part is that these two are coupled: a task-scoped policy MUST carry an expiresAt, enforced at every write boundary. That is the actual mechanism that stops a one-shot constraint from becoming permanent — an agent cannot mark a policy as tied to a specific workflow without also giving it a shelf life.

Filtering happens at read time only — active = 1 AND (expires_at IS NULL OR expires_at > ?). No sweep, no cron, no alarm (rule 47: a WHERE clause answers this for free). The row is deliberately retained and stays active, so get_policy, list_policies, and the Policies tab can still show a human that a policy existed and when it lapsed. Only the agent-injection read filters.

Implementation notes for reviewers

Three write boundaries, not two. The original enumeration listed MCP and REST and missed durable-objects/sam-session/tools/add-policy.ts — the SAM orchestrator's own add_policy. Left unfixed, every policy created from an orchestrator surface would have been permanently non-expiring: the exact failure this feature exists to remove, reintroduced through the one door nobody counted. All three now call one shared validatePolicyLifecycle, and the DO re-checks the invariant against freshly-read state immediately before the write — the choke point a future fourth writer cannot bypass (rules 44/51/61).

updatePolicy uses an explicit !== undefined check for expiresAt, not ??. null is a meaningful value here — it clears an expiry — and updates.expiresAt ?? existing.expiresAt would silently read "clear this expiry" as "leave it alone".

The scope/expiry invariant is validated against the merged post-write state, not the patch alone. Otherwise {scope:'task'} on a policy that already has an expiry would be wrongly rejected, and {expiresAt:null} on a task-scoped policy would wrongly succeed and resurrect a permanent one-shot policy.

Migration 034 is strictly additive — two ALTER TABLE ADD COLUMN, no recreation, no DROP. A Durable Object has no D1-style time-travel recovery, so a drop-and-restore here would be unrecoverable (rules 31/63).

Rule 63 enumeration (every query reading project_policies, and whether it is an authorization predicate) is recorded in the task file. None of them is: project scoping here is structural — the ProjectData DO is the project (idFromName(projectId)), so there is no project_id column that a widening could drop from a WHERE. Rule 63's failure mode does not apply, but the enumeration is recorded because the rule requires it.

A quality gate that was green for the wrong reason

Review turned up something worth calling out separately, because it is not really about this feature.

scripts/quality/check-do-migration-safety.ts extracted SQL by matching backtick and single-quoted strings only. Migration 034's second statement is double-quoted — necessarily, because it embeds DEFAULT 'always'. So that statement was never handed to the danger checks at all. The file reported PASS without it ever being inspected.

Reproduced directly:

sql.exec('ALTER TABLE project_policies ADD COLUMN expires_at INTEGER');
sql.exec("DROP TABLE project_policies");
    → DROP TABLE visible to scanner: false

Rule 31 describes this gate as one that "cannot be bypassed". A DROP TABLE on a Durable Object is unrecoverable, and the choice of quote character was silently deciding whether it got checked. The extractor now covers all three string forms, each pattern anchored to its own delimiter so a quote inside one string cannot pair with a delimiter from a neighbouring statement. Six regression tests added, proven discriminating.

This is a latent hole in a data-loss gate, unrelated to policies — it just happened to be found here because this migration was the first to use double quotes.

Not fixed — tracked instead

Expired rows are deliberately retained and no longer count toward the per-project cap, so nothing bounds total row growth (flagged independently by security-auditor and performance-reviewer, both MEDIUM). A naive total ceiling is worse than the growth it prevents: removePolicy is a soft delete, so a project that reached the ceiling could never write a policy again — a slow growth problem turned into a permanent write lockout. Sizing a ceiling requires first deciding a retention story. Documented at the cap site and tracked in tasks/backlog/2026-08-23-policy-row-retention-bound.md (rule 42 — tracked, not silent).

Part 2 — production data cleanup

Executed after this merges and deploys, using remove_policy (reversible deactivation, never a destructive delete). The deactivated ids + titles + justifications are reported in a follow-up comment on this PR and via update_task_status.

Validation

• pnpm lint (via pnpm check:fast — 0 errors; 3 pre-existing react-hooks/exhaustive-deps warnings untouched by this PR)
• pnpm typecheck — 19/19 tasks pass
• pnpm test
• Additional validation run (if applicable)
• If this PR changes candidate selection for a sweep/cron/alarm loop — N/A: no sweep/cron/alarm changed. Expiry is a read-time WHERE conjunct on an existing query; deliberately no new control loop (rule 47). git diff --stat shows zero changes under apps/api/src/scheduled/ and no alarm( additions.

Test totals, reconciled against the pre-change baseline (rule 02 — a green count is not a green suite):

I checked per-file collection status, not just assertion counts: the first API run (before building providers/cloud-init) showed 115 files failing to import, and the file total stayed at 599 across both runs — that is how I confirmed nothing silently vanished from the suite.

Discriminating proofs

Every new guard was verified by deleting it and confirming exactly the intended tests go red, then restoring:

Staging Verification (REQUIRED for all code changes — merge-blocking)

• Staging deployment green
• Live app verified via Playwright
• Existing workflows confirmed working
• New feature/fix verified on staging
• Infrastructure verification — N/A: no infra changes. No changes to packages/cloud-init/, packages/vm-agent/, DNS, TLS, or scripts/deploy/. (scripts/quality/ is a CI check, not provisioning infrastructure.)
• Mobile and desktop verification notes added for UI changes

Staging Verification Evidence

Deploy run 32673192429 — success. GET https://api.sammy.party/health → {"status":"healthy"}.

Verified against live staging (app.sammy.party / api.sammy.party), authenticated via POST /api/auth/token-login with SAM_PLAYWRIGHT_PRIMARY_USER → 200. Project 01KTKXZ4ZZAT6MJFXRW1ZTQ7RB (hono).

1. Write boundary — every rejection is live, with the real messages:

2. Merged-post-write-state validation is live (not just patch-level):

3. The read filter — an expired policy is genuinely not injected into a real agent session.

This is the acceptance criterion that matters, so I exercised the real path rather than inferring it. I created a task-scoped policy expiring in 90s plus a standing control, let the expiry lapse, then started a real chat session on staging (cf-container, "Claude Code Chat" profile) and asked the agent to call get_instructions and report which of the two it could see:

LIFECYCLE_PROBE_PRESENT=no     ← the expired policy was NOT injected
STANDING_PROBE_PRESENT=yes     ← the standing control WAS injected

The control is what makes the absence meaningful. My first attempt failed it — I had told the agent "do not call any tool", so it never fetched get_instructions and answered no to both. Without the control that would have read as a pass. Corrected prompt, re-ran, got the result above.

4. Retention — an expired policy stays inspectable: GET /policies/:id → 200 with scope:'task', active:true, lapsed:true; still present in list_policies.

5. UI, both viewports, live staging (.tmp/staging/03-policies-{desktop,mobile}.png): the expired card renders constraint + active + task-scoped + expired badges and an "Expired Aug 23, 2026" footer; the standing control card renders neither lifecycle badge nor an expiry footer. No horizontal overflow at 375×667 or 1280×800. Zero console errors.

A first-run setup wizard intermittently covered the page during this run. It silently swallowed every locator, and the "no badge on the standing card" assertions passed against a page the user never sees — the rule-62 trap. Caught by the liveness assertion, then fixed by dismissing the wizard and retrying; I also tightened liveness from "the word Policies appears" (which matched hidden nav text) to "policy cards actually rendered".

6. Regression sweep: dashboard loads and renders; project list returns 15 projects; project navigation, Agent Context tabs, and the policy filter all work; zero console errors across every page visited.

7. Cleanup: both probe policies deactivated, both test workspaces deleted ({"success":true}), and staging D1 confirms nodes are 193/193 deleted — zero VMs at rest.

UI Compliance Checklist (Required for UI changes)

• Mobile-first layout verified
• Accessibility checks completed — expiry/scope state is conveyed by text (expired badge, "Expired <date>" footer), not by colour alone; colour is supplementary emphasis
• Shared UI components used — reuses the existing Badge and the same semantic tokens (bg-info-tint/text-info-fg, bg-warning-tint/text-warning-fg) already used by the category badges. No new dependency, no bundle impact; AgentContextPage is already React.lazy-loaded.
• Playwright visual audit run locally

The audit covers the standing / live-task-scoped / expired / worst-case matrix at 375×667, 390×844, 768×1024 and 1280×800, using assertNoOverflow from audit-helpers.ts (which includes the clipped-overflow walk — rule 56), not a hand-rolled documentElement.scrollWidth check.

Review caught that the worst case was split across two cards: pl4 carried the 200-character unbroken title but was not expired, so "longest title" and "most badges" never met on one card. pl4 is now the genuine worst case — 200-char title carrying every badge (category + active + task-scoped + expired) — and both viewport tests assert the badges on that card rather than page-wide (page-wide would now be a strict-mode violation, not a pass).

Screenshots were opened and checked, not merely produced (rule 62): the four viewport sizes produce visibly different files (286KB / 375KB / 691KB / 723KB), and the 375px capture confirms the standing-policy control renders with no lifecycle badge while the task-scoped card shows the badge and its title wraps cleanly.

End-to-End Verification (Required for multi-component changes)

• Data flow traced from user input to final outcome with code path citations
• Capability test exercises the complete happy path across system boundaries
• All spec/doc assumptions about existing behavior verified against code
• If any gap exists between automated test coverage and full E2E, manual verification steps documented below

Data Flow Trace

1. Agent calls add_policy with scope:'task' + expiresAt
   → apps/api/src/routes/mcp/policy-tools.ts:handleAddPolicy()
   → parseLifecycleParams() (type shape) → validatePolicyLifecycle() (the invariant)

2. Service layer threads the two new params
   → apps/api/src/services/project-data-policies.ts:createPolicy()

3. DO RPC
   → apps/api/src/durable-objects/project-data/index.ts:createPolicy()

4. Pure SQL — final server-side guard, then INSERT
   → apps/api/src/durable-objects/project-data/policies.ts:createPolicy()
   → cap COUNT uses APPLIES_NOW_SQL so expired rows do not consume headroom

5. Later session opens and calls get_instructions
   → apps/api/src/routes/mcp/instruction-tools.ts:handleGetInstructions()
   → projectDataService.getActivePolicies()
   → policies.ts:getActivePolicies() — WHERE active = 1 AND (expires_at IS NULL OR expires_at > ?)
   → an expired policy is NOT returned, so it costs zero tokens

6. Surviving policies render, with the annotation
   → instruction-tools.ts:formatPolicyLifecycle() → formatPolicyDirectives()
   → "- **Title** (task-scoped, expires 2026-09-30) (id: <full uuid>): content"

7. Human sees why a policy stopped applying
   → row-schemas/policies.ts:parsePolicyRow() → listPolicies()
   → apps/web/src/pages/AgentContextPage/PoliciesTab.tsx — task-scoped / expired badges

The R1 merge point is step 6. R1 moved the policy id inline into the rendered line; this PR inserts the lifecycle annotation before it. The pre-existing "renders ids in FULL" test lives in an untouched sibling file (mcp-instruction-payload-dedup.test.ts) and was re-run: 14/14 pass. When formatPolicyLifecycle returns '' — which it does for every policy without an expiry, i.e. all 81 today — the line collapses back to byte-identical pre-change output.

Untested Gaps

The DO's fresh-read guard is not exercised by two genuinely concurrent updatePolicy RPCs. This is not the rule-45 class of bug: the DO's read → guard → write critical section contains no await, so it completes within one JS turn and two RPCs to the same DO instance cannot interleave across it. An explicit Promise.all test would be documentation rather than a correctness guard.

Part 2 (production policy deactivation, acceptance criterion 8) is by design not actionable until this deploys. Results reported in a follow-up comment.

Post-Mortem (Required for bug fix PRs)

This is a feature PR, but it carries one genuine bug fix — the migration-safety gate — so the post-mortem covers that.

What broke

pnpm quality:do-migration-safety reported migrations.ts: PASS while never inspecting one of the statements in the file. Any DROP TABLE, DELETE without WHERE, or UPDATE without WHERE written in a double-quoted string would have shipped through a green CI gate. On a Durable Object that is unrecoverable — there is no D1-style time travel.

Root cause

extractSqlFromTypeScript (scripts/quality/check-do-migration-safety.ts) matched only two of JavaScript's three string forms: backtick and single-quoted. Double-quoted strings were never extracted, so they were never scanned. Double quotes are not an exotic choice — they are the natural choice for a statement that embeds a single-quoted SQL literal, which is why migration 034 (… DEFAULT 'always') was the first migration to trip it.

Class of bug

A checker whose "pass" is the absence of input rather than the absence of danger — the same shape as rule 02's "a green test count is not a green suite" and rule 53's "silence is not success". A linter run on a path that matched no files, a coverage gate over an empty file set, and a SQL scanner that skipped a quoting style all report identically to a genuine pass. It is also a validated-input bug: the extractor silently defined what "all the SQL in this file" means, and nothing checked that definition against reality.

Why it wasn't caught

The gate's own test suite tested the checker's verdict on the real codebase (expect(result).toContain('PASS')) and re-implemented its own regex scans over the migration files — but never tested the extractor against the string forms it claims to cover. Every test agreed with the extractor's blind spot because every test shared it. No test had ever fed the checker a known-dangerous statement and demanded it be caught.

Process fix included in this PR

• scripts/quality/check-do-migration-safety.ts — extractor now covers all three string forms, each pattern anchored to its own delimiter, with a comment explaining that a quoting style must never be able to bypass a gate rule 31 calls unbypassable. Exported for direct testing.
• scripts/quality/check-do-migration-safety.test.ts — six tests that feed the extractor each string form directly, including the exact shape of migration 034 and an end-to-end "a DROP TABLE hidden in a double-quoted statement is still caught". Verified discriminating.
• apps/api/src/durable-objects/migrations.ts — the migration-034 comment warning authors about quoted SQL in prose updated; it described the old backtick-only behaviour.

The generalisable lesson — test the checker by giving it something it must catch, not only by checking that it likes the current codebase — is already stated in .claude/rules/02-quality-gates.md ("A Green Test Count Is Not A Green Suite", which explicitly extends to "schema checks that skip when the input is unparseable"). This is that rule's first concrete instance in the migration-safety tooling, so the fix is the test, not a new rule.

Post-mortem file

This section, plus tasks/active/2026-08-23-policy-lifecycle-controls.md.

Specialist Review Evidence (Required for agent-authored PRs)

• All local reviewers completed and findings addressed before merge
• If any reviewer did NOT complete: needs-human-review label added and merge deferred to human — N/A, all ten completed

All findings addressed in c06642e8a unless noted.

Exceptions (If any)

• Scope: total project_policies row growth is not bounded by this PR.
• Rationale: a naive total ceiling would permanently lock a project out of writing policies, because removePolicy is a soft delete. Bounding growth needs a retention/hard-delete design first.
• Expiration: tracked in tasks/backlog/2026-08-23-policy-row-retention-bound.md.

Agent Preflight (Required)

• Preflight completed before code changes

Classification

• external-api-change
• cross-component-change
• business-logic-change
• public-surface-change
• docs-sync-change
• security-sensitive-change
• ui-change
• infra-change

External References

N/A: no external API involved. This is entirely internal — SQLite DDL against Cloudflare Durable Object storage plus SAM's own MCP/REST surfaces. SQLite's ALTER TABLE ADD COLUMN constraints were verified against ~10 prior migrations in the same file that already use the NOT NULL DEFAULT '<literal>' idiom successfully against real workerd, and confirmed by the real-DO test run rather than assumed.

Codebase Impact Analysis

• packages/shared — types/policy.ts (POLICY_SCOPES, PolicyScope, isPolicyScope, new fields on 3 interfaces), constants/policies.ts (DEFAULT_POLICY_MAX_EXPIRY_MS, maxExpiryMs, the shared validatePolicyLifecycle)
• apps/api — DO migration 034; project-data/policies.ts, project-data/index.ts, row-schemas/policies.ts; services/project-data-policies.ts; MCP policy-tools.ts + tool-definitions-policy-tools.ts + instruction-tools.ts; REST routes/policies.ts + schemas/policies.ts; sam-session/tools/add-policy.ts; env.ts
• apps/web — AgentContextPage/PoliciesTab.tsx (display-only badges + footer)
• apps/www — architecture/overview.md
• scripts/quality — check-do-migration-safety.ts (+ tests)

Documentation & Specs

• apps/www/src/content/docs/docs/architecture/overview.md — documented the lifecycle annotation in the get_instructions bootstrap payload and the read-time filter semantics
• CLAUDE.md — Recent Changes entry
• tasks/active/2026-08-23-policy-lifecycle-controls.md — research, rule-63 query enumeration, three-writer enumeration, checklist, acceptance criteria
• tasks/backlog/2026-08-23-policy-row-retention-bound.md — new, tracks the deferred growth bound

Constitution & Risk Check

Principle XI (No Hardcoded Values): DEFAULT_POLICY_MAX_EXPIRY_MS (365 days) with a POLICY_MAX_EXPIRY_MS override, threaded through resolvePolicyLimits and reaching all three write boundaries. Registered in env.ts alongside the five existing POLICY_* vars.

Principle XIII (Fail Fast): every write boundary fails closed on a task scope with no expiry, and the DO re-checks against freshly-read state as the final guard.

Risks and tradeoffs:

1. A wrong expiry silently drops a policy from agent context. Mitigated by bounding expiresAt to the future and within maxExpiryMs, by retaining the row so a human can see it lapsed, and by surfacing expired in the UI. Accepted: this is the feature working as intended.
2. Migration risk. Additive only; both defaults reproduce pre-migration behaviour exactly, so all 81 existing rows are unaffected by construction. Verified against the real migration chain in workerd.
3. Unbounded row growth. Accepted and tracked — see Exceptions.
4. The R1 merge conflict. Three overlapping spans in instruction-tools.ts, resolved; the untouched full-uuid test re-run as the check.